docs: list amp as a built-in target in the architecture and diagnostics references - #758
Conversation
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6b92e55a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | `AB3008` | error | The Skill Markdown body uses a canonical token (`agent-bundle:token:arguments`, `agent-bundle:path:plugin-root`, `agent-bundle:path:plugin-data`, `agent-bundle:path:workspace-root`, `agent-bundle:token:session-identity`, `agent-bundle:token:skill-root`) or a host alias of one (`$ARGUMENTS`, `${CLAUDE_PLUGIN_ROOT}`, `${PLUGIN_ROOT}`, `${workspaceFolder}`, …), and the skill lowers to a host whose pinned Skill Markdown contract documents no interpolation placeholder: Codex, Cursor, portable, and the unified `plugin` target's shared portable document. Claude documents all six. One diagnostic per token per host: `Skill token "<token>" has no <host> Skill Markdown equivalent.` | Remove the token, restrict the skill to a host that documents it, or move the reference to a document that host interpolates. | | ||
| | `AB3009` | error | After token lowering, the `<host>` Skill Markdown body still contains a placeholder spelling another host's contract owns — for Codex, Cursor, and portable documents, one of Claude's `$ARGUMENTS`, `${CLAUDE_PLUGIN_DATA}`, `${CLAUDE_PLUGIN_ROOT}`, `${CLAUDE_PROJECT_DIR}`, `${CLAUDE_SESSION_ID}`, `${CLAUDE_SKILL_DIR}`; the message names the syntax. | Use canonical agent-bundle tokens so lowering emits only this host's documented placeholders. | | ||
| | `AB3010` | error | The lowered `<host>` skill frontmatter fails that host's pinned schema — `schemas/skill-hosts/claude-skill-frontmatter.schema.json` and `cursor-skill-frontmatter.schema.json` for Claude and Cursor, the Agent Skills frontmatter schema for Codex and portable: `Lowered <host> Skill document <field> <message>.` | Remove the unsupported field or restrict the skill to a host that documents it. | | ||
| | `AB3006` | error | Skill frontmatter declares a field that is not a portable Agent Skills field (`name`, `description`, `license`, `compatibility`, `allowed-tools`, `metadata`), not a typed host extension (Claude: `agent`, `argument-hint`, `arguments`, `background`, `context`, `disallowed-tools`, `effort`, `hooks`, `model`, `shell`, `user-invocable`, `when_to_use`; Cursor: `color`, `globs`, `icon`; Amp: `builtin-tools`, `mcpServers`; shared: `disable-model-invocation`, `paths`), and not the authoring key `targets`; or `targets` is not an object, carries a key other than `amp`, `claude`, `codex`, `cursor`, or holds an unknown key inside `targets.<host>` (including `targets.amp`, `targets.codex.interface`, `.policy`, `.dependencies`, and `.dependencies.tools[<n>]`). The message names the field path. | Move host-only fields into `targets.<host>` or a documented host key, or remove the unknown field. | |
There was a problem hiding this comment.
Remove valid
targets.amp from the invalid examples
For a Skill using the newly supported targets: { amp: ... } extension, this row now simultaneously lists amp as an accepted target key and names targets.amp as an example of an unknown field. skills/parse-ir.ts explicitly accepts amp and reports only unknown keys nested beneath it, so the generated diagnostics reference incorrectly implies that valid Amp configuration triggers AB3006; remove this example or replace it with a genuinely invalid nested key.
AGENTS.md reference: AGENTS.md:L139-L142
Useful? React with 👍 / 👎.
Closes #755. Closes #756.
Amp shipped as a built-in adapter in #729, but two documentation contracts still described the
four-adapter registry that preceded it, and both feed public pages.
website/docs/{en,zh}/guide/concepts/architecture.mdx(hand-written)ampjoins the planner table, namingadapters/amp.tsand its pinnedadapters/capabilities/amp-0.0.0-20260907001852-gf348fed.json.createDefaultRegistryprose now includesampand says what registration order is not:planCompositesorts the selection before planning, and the composite identity is the sortedname join.
portablestays the default;pluginstays a non-target.index.jsPluginAPI factory under
.amp/plugins/{plugin}/that registers Skills and callbacks throughthe API, and skill-scoped MCP (exactly one Amp Skill for a canonical server; a compiler-owned
local server refused for want of a documented plugin-root placeholder). The per-surface
evidence stays on the generated Hosts page, which the paragraph links, along with the
author-facing capability map from docs: complete convention-first guides, targets, and capability navigation (EN/ZH) #754.
BundleIdentityHostis corrected toamp | claude | codex | cursorwith the note thatDoctorHostexcludes Amp.docs/diagnostics.md(the source the site's diagnostics reference is rendered from)AB4100: the built-in registry statement and the recovery list both nameamp.AB3006(Amp'sbuiltin-tools/mcpServersextension fields andtargets.amp),AB3008(Amp documents no Skill Markdown interpolation engine, so it belongs with Codex, Cursor, and
portable),
AB3009(foreign-placeholder detection covers Amp documents too),AB3010(Ampvalidates the Agent Skills frontmatter schema plus its own skill MCP rules), the composite-root
section,
AB4106, andAB6023. The stale "unifiedplugintarget's shared document" clauseswent with them.
docs/diagnostics.mdline 239 keeps its four host-scoped config keys deliberately: the Ampadapter declares no
configExtension, soclaude,codex,cursor,portableis thecomplete list there, matching
reference/configuration.mdx.No generated page was hand-edited —
website/docs/{en,zh}/reference/diagnostics.mdis renderedfrom
docs/diagnostics.mdat build time, and the built output was checked to confirm theAB4100 row now lists all five targets in both locales.
New guard (
packages/agent-bundle/tests/diagnostics-doc-targets.test.ts): the AB4100 row'strigger and recovery target lists are parsed out of
docs/diagnostics.mdand compared withcreateDefaultRegistry().names(), so the next built-in adapter fails a test instead of silentlydrifting the public reference. Falsified before landing: removing
ampfrom the row fails thetest with the target diff.
Docs and a test only — no publishable package source changed, so
pnpm changeset status --since=origin/mainrequires nothing (skip-changeset).Local gate
Run on
f6b92e55a3, which contains currentorigin/main(361d27ff5e):pnpm buildpnpm typecheckpnpm lintpnpm test:unitpnpm docs:site:buildnpx rstest --config rstest.unit.config.ts run packages/agent-bundle/tests/diagnostics-doc-targetsDeslop
Deslop: claude-fable-5-1-thinking-xhigh, 2 edits — trimmed an eight-line doc comment in the
guard test that narrated issue history, and removed the stale "unified
plugintarget's shareddocument" clauses the Amp rows inherited.
Self-review
Reviewer:
claude-fable-5-1-thinking-xhigh, thengpt-5.6-sol-maxon the fix commit.ampto the composite-root paragraph made it claim Amp reads the artifact directory as its plugin root and sharesskills/,hooks/,mcp/,scripts/,bin/,INSTALL.md. Amp's root is the nested.amp/plugins/<plugin>/directory, self-contained down to its ownskills/andhooks/, andinstall ampcopies only that subdirectoryac38b42ac7: the shared-root sentence now coversclaude,codex,cursor,portable, and Amp gets its own exception sentenceac38b42ac7`([a-z]+)`name pattern would miss a future hyphenated or numeric target nameAB3010's Agent Skills schema sees only the six portable keys, sobuiltin-toolsis not schema-validated at loweringbuiltin-toolsis schema-checked